fix(pgsql-test): fail the harness when seeding fails - #1774
Merged
Conversation
Contributor
🤖 Devin AI EngineerI'll be helping with this pull request! Here's what you should know: ✅ I will automatically:
Note: I can only respond to comments from users who have write access to this repository. ⚙️ Control Options:
|
|
Review complete. No issues found — approved ✅. This PR hardens the pgsql-test harness so that when seeding fails, the partially-created test database and its connection pool are cleaned up before the seed error is rethrown, and it adds tests for that seed-failure path.
Reviewed commit: 2fcfd0c |
This comment has been minimized.
This comment has been minimized.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
getConnectionscaught every error from the seed phase, logged it to stderr, and handed the caller working clients on a half-built database:So a failed fixture deploy — the pgpm deploy that stands the schema up, or any
seed.fn/seed.sqlfile— is not a setup failure but a passing setup, and the suite reportsrelation "…" does not existin each test instead of the one error that caused it. Jest buries the stderr line above the first failure, which is how a single broken deploy reads as twenty unrelated mysteries.Now the seed phase is fatal: stderr formatting is kept, the ephemeral database is torn down (its own failure deliberately swallowed with a comment, so it can't mask the seed error), and the original error is rethrown as the
cause:Two tests cover it (a throwing adapter and invalid seed SQL). Measured before changing: no suite in this package, nor the db-heavy suites in
constructive-db(metaschema, introspection, database-jobs, app provisioning), was riding on a swallowed seed failure — patched and unpatched runs are identical, and the rethrow was verified to actually fire with a deliberately-throwing adapter.Link to Devin session: https://app.devin.ai/sessions/47477486a4fd45e684bd663b7007a629
Requested by: @pyramation